fix(ci): run PR quality gates on every base branch, not just main - #1447
Conversation
A `pull_request.branches:` allowlist filters on the PR's *base* ref. Six gates carried `branches: [main]` (or `[main, develop]`), so a PR stacked onto another PR's branch matched none of them and ran with no CI, no CodeQL, no security scan, no coverage, no e2e and no dependency review. This is not hypothetical. #1440 -- "fix(web): stop /api/transcribe leaking probe status and key config", a non-draft security fix -- targets #1381's branch and has zero CI runs. Its only green checks are the Vercel deployment and CodeRabbit. Every test gate in the repo silently vanished because of the base it happens to target. Dropping the allowlist from the `pull_request` trigger makes each gate fire for every PR regardless of base. The `push` triggers are untouched and stay pinned to main, so branch pushes do not start running the full suite twice. e2e-tests.yml keeps its existing `vars.E2E_BASE_URL` job-level guard, so its PR behaviour is unchanged apart from now being reachable at all. Non-vacuous: tests/unit/test_workflow_pr_gate_coverage.py run against the pre-fix workflows from origin/main gives 6 failed, 12 passed -- one failure per gate, exactly the allowlist assertion. Against the fix: 18 passed. The push-trigger test is what fails if a future edit widens the `push` scope while removing the PR filter. Verified: 27 passed across the two workflow-YAML test modules (test_pr_governance_workflow.py unchanged at 9); all 32 workflow files still parse as YAML. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ubMueWZScQy2KgsQ4uHQi
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
|
@coderabbitai full review Requesting manually because auto-review skipped this PR — which is itself a data point for the blocker described in the PR body. The skip notice above names the inherited gate directly:
This PR opened with no labels, so it matched none of the 26 and was skipped. That is the exact deadlock #1425 diagnoses and fixes with Generated by Claude Code |
|
|
* fix(ci): add concurrency groups so the Actions queue can drain The repository's Actions queue is saturated and not draining: 285 runs queued, the oldest sitting unstarted for ~24 minutes with only 2 runs in progress. Pushes to `main` are affected as well as PRs, so no PR can reach a green CI gate and nothing is mergeable. Cause: 17 of 30 workflows carried no `concurrency:` block, including the heaviest and most-triggered ones (ci, codeql-analysis, security, e2e-tests, pr-checks, auto-label, dependency-review). #1447 correctly removed their base-branch filters so every PR runs the gate, but without a concurrency group each push stacks a brand-new full run beside the runs it obsoletes. With ~30 workflows and dozens of open PRs the arrival rate exceeds the drain rate and the backlog grows without bound. Add a concurrency group to the 14 workflows that trigger on pull_request, pull_request_target, push, or issues: - Group is keyed on the PR number where one exists, falling back to github.ref. Keying on github.ref alone would be wrong for the pull_request_target workflows (pr-checks, dependabot-auto-merge), where github.ref resolves to the base branch and every PR would collide into a single group and cancel its siblings. - cancel-in-progress is true only for pull_request and pull_request_target events. Push and schedule runs queue rather than cancel, so each merged commit keeps its own verdict instead of being cancelled by the next merge. - auto-label triggers on both PRs and issues, so its key falls through pull_request.number -> issue.number -> github.ref. Left untouched: bulk-issue-processor, real-processing, and stale, which are workflow_dispatch- or schedule-only and are not queue pressure. Group names are prefixed by workflow slug and do not collide with the existing groups in coverage, secret-scan, pr-governance, verification, phase-goal-tracker, or the gh-aw lock files. This changes scheduling only. No job, step, trigger, or permission is modified; all 28 workflow files still parse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FoRdXUHXgtt3U6HiHdcosV * fix(ci): key non-PR concurrency by SHA so merge bursts keep their verdicts Self-review correction to the previous commit, plus the same fix applied to two workflows that already had the defect. The previous commit keyed every group on `github.event.pull_request.number || github.ref` and set `cancel-in-progress` false for push events, on the reasoning that push runs would queue rather than cancel and so "each merged commit keeps its own verdict". That claim is wrong. `cancel-in-progress: false` prevents a new run from cancelling a *running* one. It does not prevent GitHub from cancelling a *pending* one: when a run is queued into a group that already has one in progress and one pending, the pending run is cancelled. Because `github.ref` is `refs/heads/main` for every merge, all main pushes shared one group, so a burst of merges -- exactly what this repository does, ten in twenty minutes -- would have silently dropped the middle commits' verdicts. That is a quieter version of the failure being fixed. Key non-PR runs on `github.sha` instead. Each push and scheduled run then lands in its own singleton group and can neither cancel nor be cancelled. This costs nothing: every `push:` trigger in these workflows is already filtered to `main` (verified across all 13), so pushes to PR branches fire no push runs at all and all queue pressure comes from `pull_request`/`pull_request_target` events, which still cancel their predecessors by PR number. The conditional `cancel-in-progress` is kept as a second, explicit safeguard. Also apply the same key to coverage.yml and secret-scan.yml. Both predate this change and both carried the bare `github.ref` group with `cancel-in-progress: true` -- the more aggressive form of the same bug, where a merge to main cancels the previous merge's coverage and secret scan outright. Leaving them as-is while documenting the hazard in fourteen other files would have been incoherent. Issue-triggered workflows (auto-assign, issue-triage, auto-label) keep their `github.event.issue.number` key; `github.sha` is meaningless for issues events and would collide every issue into one group. Scheduling only. No job, step, trigger, or permission changed; all 28 workflow files parse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FoRdXUHXgtt3U6HiHdcosV * fix(ci): key dependabot-auto-merge check_suite runs by head SHA Review finding from the Vercel review bot on #1510, confirmed and applied. dependabot-auto-merge is the one workflow here that triggers on check_suite, and on that event GITHUB_SHA is the tip of the default branch rather than the commit under test. It is therefore identical for every PR, so the `|| github.sha` fallback collapsed all concurrent check-suite runs into a single group. Since cancel-in-progress is false for that event, they would not cancel a running peer -- but GitHub still cancels the *pending* run in a group, so one PR's merge-gate verdict could be dropped by another PR's check suite arriving. Fall back to github.event.check_suite.head_sha first, which is the PR head and keeps the runs isolated, before github.sha. Audited the other events reached by these groups for the same class of degeneracy: - issues (auto-assign, auto-label, issue-triage) already key on github.event.issue.number, which is unique. - schedule (codeql-analysis, security) resolves github.sha to the default branch tip, which can coincide with that same workflow's push run at the same commit. That collision is between two runs of the same scan over the same tree, so collapsing them is harmless dedup rather than a lost verdict. check_suite was the only real case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FoRdXUHXgtt3U6HiHdcosV --------- Co-authored-by: Claude <noreply@anthropic.com>
Canonical issue
Closes # (no pre-existing issue — found during the PR remediation sweep; happy to file one if the repo requires a canonical issue before merge)
Outcome
Every PR gets the full gate suite regardless of which branch it targets. Today a PR stacked onto another PR's branch runs no CI, no CodeQL, no security scan, no coverage, no e2e, and no dependency review — because all six workflows filter
pull_requeston the base ref withbranches: [main].This is live right now: #1440 ("fix(web): stop /api/transcribe leaking probe status and key config") is a non-draft security fix targeting #1381's branch, and it has zero CI runs. Its only green checks are the Vercel deployment and CodeRabbit. Confirmed against the last 30
ci.ymlruns — its head branch (claude/clever-heisenberg-cjf3tn) appears in none of them.Scope
pull_requesttrigger inci.yml,security.yml,coverage.yml,e2e-tests.yml,codeql-analysis.yml,dependency-review.yml; addedtests/unit/test_workflow_pr_gate_coverage.py.pushtriggers (unchanged, still pinned tomain/main,develop).verification.ymlis pinned topull_request.branches: [refactor/hybrid-infra-v2]and appears to be dead — left alone rather than arming a heavy gate as a side effect.secret-scan.yml,pr-checks.ymlandpr-governance.ymlalready had no base filter.Risk
mainbases, which previously ran nothing. Thepushtriggers were deliberately left scoped so branch pushes don't double-run.e2e-tests.ymlkeeps its job-levelvars.E2E_BASE_URLguard, so its PR behaviour is unchanged apart from being reachable at all.Verification
Head SHA
1e5e0b5.tests/unit/test_workflow_pr_gate_coverage.py: 18 passed. Combined with the existingtest_pr_governance_workflow.py: 27 passed.origin/main: 6 failed, 12 passed, one failure per gate, exactly thetest_gate_has_no_base_branch_allowlistassertion. Thetest_push_trigger_stays_scoped_to_maincase is what catches a future edit that widenspushwhile removing the PR filter.Note on local scope:
tests/unit/has 61 pre-existing collection errors in this sandbox from uninstalled project dependencies (pip install -e .[dev]was never run here). They are unrelated to this diff, which touches only workflow YAML plus one new test module.Production evidence
Not applicable — CI configuration only, no runtime code paths changed.
Agent handoff
Related blocker found in the same sweep (not fixed here)
CodeRabbit is reviewing nothing in this repo. Every recent PR carries the commit status
CodeRabbit — Review skipped: excluded by label configuration(#1381, #1410, #1420, #1439, #1443, #1445) orReview rate limited(#1423, #1440). An inherited required-labels gate from the org/dashboard config blocks auto-review until a label is applied, but PRs open unlabelled and labelling afterwards does not retro-trigger.#1425 already diagnoses this correctly and fixes it with
reviews.auto_review.labels: []in.coderabbit.yaml— but #1425 is still a draft, so the fix for "reviews never start" is itself waiting on a review that will never start. Marking #1425 ready and merging it unblocks the review loop for every other open PR.Generated by Claude Code